Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat/Announce capacity via the attributes #2495

Merged
merged 1 commit into from
Aug 15, 2023
Merged

feat/Announce capacity via the attributes #2495

merged 1 commit into from
Aug 15, 2023

Conversation

carpawell
Copy link
Member

It is done automatically (as a sum of existing space on every file system available on the configured shards) but can be overwritten if Capacity attribute is provided via the application configuration. Closes #602.

@carpawell carpawell self-assigned this Aug 10, 2023
@carpawell
Copy link
Member Author

Let's briefly review it and then decide what we are doing with:

  1. windows;
  2. the fact that announcing space will never be used fully in practice.

@codecov
Copy link

codecov bot commented Aug 10, 2023 •
edited
Loading

Codecov Report

Merging #2495 (688773c) into master (aba6622) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

❗ Current head 688773c differs from pull request most recent head 7c54307. Consider uploading reports for the commit 7c54307 to get more accurate results

@@            Coverage Diff             @@
##           master    #2495      +/-   ##
==========================================
- Coverage   29.70%   29.68%   -0.03%     
==========================================
  Files         403      404       +1     
  Lines       30670    30697      +27     
==========================================
  Hits         9111     9111              
- Misses      20794    20821      +27     
  Partials      765      765
Files Changed Coverage Δ
cmd/neofs-node/config.go 0.00% <0.00%> (ø)
cmd/neofs-node/fs_unix.go 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@carpawell carpawell force-pushed the feat/add-capacity-attribute-automatically branch from 224961c to 99fd661 Compare August 10, 2023 22:05
roman-khimov
roman-khimov reviewed Aug 11, 2023
View reviewed changes
Copy link
Member

@roman-khimov roman-khimov left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add win-compatible code as well: https://stackoverflow.com/questions/20108520/get-amount-of-free-disk-space-using-go

cmd/neofs-node/config.go Show resolved Hide resolved
cmd/neofs-node/config.go Outdated
}
}

// according to API description, in GB, not GiB
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it's obviously 1024^3. We do not use iB notation and likely won't be.

Copy link
Member Author

@carpawell carpawell Aug 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so make 1 << 30 instead of 1000*1000*1000?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

cmd/neofs-node/config.go Show resolved Hide resolved
@carpawell
Copy link
Member Author

carpawell commented Aug 11, 2023 •
edited
Loading

We can add win-compatible code as well: https://stackoverflow.com/questions/20108520/get-amount-of-free-disk-space-using-go

Sure we can, but do we need it? We have not announced Windows support yet. Every time I do such a task I have to boot Windows and test it. It is not a problem but I want to read "we try our best for Windows too" explicitly.

@carpawell
Copy link
Member Author

@roman-khimov, also, please, see the comment (in case you missed it).

@roman-khimov
Copy link
Member

@roman-khimov, also, please, see the comment (in case you missed it).

That's exactly why Windows was mentioned at all, it's not that hard to add support here. The second question is just not really a problem.

cthulhu-rider
cthulhu-rider reviewed Aug 11, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@carpawell carpawell marked this pull request as draft August 14, 2023 09:35
@carpawell carpawell force-pushed the feat/add-capacity-attribute-automatically branch from 99fd661 to 9ec51a1 Compare August 14, 2023 17:39
@carpawell carpawell marked this pull request as ready for review August 14, 2023 17:39
@carpawell carpawell force-pushed the feat/add-capacity-attribute-automatically branch 2 times, most recently from bf32f1d to 3eda9ed Compare August 14, 2023 17:45
cthulhu-rider
cthulhu-rider reviewed Aug 15, 2023
View reviewed changes
cmd/neofs-node/config.go Outdated Show resolved Hide resolved
cmd/neofs-node/fs_windows.go Outdated Show resolved Hide resolved
@carpawell carpawell force-pushed the feat/add-capacity-attribute-automatically branch from 3eda9ed to 15abfb6 Compare August 15, 2023 12:20
@roman-khimov roman-khimov mentioned this pull request Aug 15, 2023
Closed
@roman-khimov
Copy link
Member

Conflicts.

@carpawell
node: Announce capacity via the attributes
7c54307 
It is done automatically (as a sum of existing space on every file system
available on the configured shards) but can be overwritten if `Capacity`
attribute is provided via the application configuration.
Closes nspcc-dev#602.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@carpawell carpawell force-pushed the feat/add-capacity-attribute-automatically branch from 15abfb6 to 7c54307 Compare August 15, 2023 15:36
@roman-khimov roman-khimov merged commit 8a37926 into nspcc-dev:master Aug 15, 2023
7 of 8 checks passed
@carpawell carpawell deleted the feat/add-capacity-attribute-automatically branch August 15, 2023 17:35
roman-khimov added a commit that referenced this pull request Sep 13, 2023
@roman-khimov
node: don't mkdir random paths from configuration
74dc2b3 
Unbreak peapods:
  2023/09/13 18:50:36 open shard S4wpuCnzpWW7SbwhER3U1v: could not open *blobstor.BlobStor: open substorage peapod: open BoltDB instance: open /storage/peapod0.db: is a directory

Call `stat()` gently instead walking up. FS mount point has to exist there in
any event and we should have some access to it.

The real problem is that #2462 (introducing Peapod) was correct on its own.
And #2495 (introducing capacity) was also correct on its own. But they don't
work together.

Refs 7c54307.
Refs c060b16.

util.MkdirAllX will be removed from code in most of the cases.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
roman-khimov added a commit that referenced this pull request Sep 14, 2023
@roman-khimov
node: don't mkdir random paths from configuration
c75f18b 
Unbreak peapods:
  2023/09/13 18:50:36 open shard S4wpuCnzpWW7SbwhER3U1v: could not open *blobstor.BlobStor: open substorage peapod: open BoltDB instance: open /storage/peapod0.db: is a directory

Call `stat()` gently instead walking up. FS mount point has to exist there in
any event and we should have some access to it.

The real problem is that #2462 (introducing Peapod) was correct on its own.
And #2495 (introducing capacity) was also correct on its own. But they don't
work together.

Refs 7c54307.
Refs c060b16.

util.MkdirAllX will be removed from code in most of the cases.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
roman-khimov added a commit that referenced this pull request Sep 14, 2023
@roman-khimov
node: don't mkdir random paths from configuration
47bb163 
Unbreak peapods:
  2023/09/13 18:50:36 open shard S4wpuCnzpWW7SbwhER3U1v: could not open *blobstor.BlobStor: open substorage peapod: open BoltDB instance: open /storage/peapod0.db: is a directory

Call `stat()` gently instead walking up. FS mount point has to exist there in
any event and we should have some access to it.

The real problem is that #2462 (introducing Peapod) was correct on its own.
And #2495 (introducing capacity) was also correct on its own. But they don't
work together.

Refs 7c54307.
Refs c060b16.

util.MkdirAllX will be removed from code in most of the cases.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cthulhu-rider cthulhu-rider cthulhu-rider approved these changes

@roman-khimov roman-khimov roman-khimov approved these changes

@AliceInHunterland AliceInHunterland Awaiting requested review from AliceInHunterland

Assignees

@carpawell carpawell

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Capacity attribute in testnet config
3 participants
@carpawell @roman-khimov @cthulhu-rider